From b8158cfdc8dd3b197e43ec85643aa6e80a5b1af1 Mon Sep 17 00:00:00 2001 From: Jim McGrath Date: Tue, 2 May 2017 10:34:29 -0500 Subject: [PATCH] fix dynamic library search path for build scripts Make dynamic library search path handling for build scripts mirror the behaviour for cargo run etc. -L paths are taken and stripped of the native= and similar prefixes and added to the dynamic library search path if they are inside the target dir. Resolves https://github.com/rust-lang/cargo/issues/3957 --- src/cargo/ops/cargo_rustc/compilation.rs | 29 ++------------ src/cargo/ops/cargo_rustc/context.rs | 5 +++ src/cargo/ops/cargo_rustc/custom_build.rs | 5 ++- src/cargo/ops/cargo_rustc/mod.rs | 46 ++++++++++++++++++++--- tests/build-script.rs | 25 ++++++++---- tests/plugins.rs | 33 ++++++++++------ 6 files changed, 93 insertions(+), 50 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs index 5192d2cad..286c0edc9 100644 --- a/src/cargo/ops/cargo_rustc/compilation.rs +++ b/src/cargo/ops/cargo_rustc/compilation.rs @@ -100,32 +100,9 @@ impl<'cfg> Compilation<'cfg> { let mut search_path = if is_host { vec![self.plugins_dylib_path.clone()] } else { - let mut search_path = vec![]; - - // Add -L arguments, after stripping off prefixes like "native=" - // or "framework=" and filtering out directories *not* inside our - // output directory, since they are likely spurious and can cause - // clashes with system shared libraries (issue #3366). - for dir in self.native_dirs.iter() { - let dir = match dir.to_str() { - Some(s) => { - let mut parts = s.splitn(2, '='); - match (parts.next(), parts.next()) { - (Some("native"), Some(path)) | - (Some("crate"), Some(path)) | - (Some("dependency"), Some(path)) | - (Some("framework"), Some(path)) | - (Some("all"), Some(path)) => path.into(), - _ => dir.clone(), - } - } - None => dir.clone(), - }; - - if dir.starts_with(&self.root_output) { - search_path.push(dir); - } - } + let mut search_path = + super::filter_dynamic_search_path(self.native_dirs.iter(), + &self.root_output); search_path.push(self.root_output.clone()); search_path.push(self.deps_output.clone()); search_path diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 0dc584ccd..9e81a9e64 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -336,6 +336,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> { self.host.deps() } + /// Return the root of the build output tree + pub fn target_root(&self) -> &Path { + self.host.dest() + } + /// Returns the appropriate output directory for the specified package and /// target. pub fn out_dir(&mut self, unit: &Unit) -> PathBuf { diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 5b7e52c19..4c1dca592 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -181,6 +181,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) fs::create_dir_all(&script_output)?; fs::create_dir_all(&build_output)?; + let root_output = cx.target_root().to_path_buf(); + // Prepare the unit of "dirty work" which will actually run the custom build // command. // @@ -218,7 +220,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) } if let Some(build_scripts) = build_scripts { super::add_plugin_deps(&mut cmd, &build_state, - &build_scripts)?; + &build_scripts, + &root_output)?; } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index f52f86860..439046eb1 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -297,6 +297,8 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc) -> CargoResult) -> CargoResult Option> { // execute. fn add_plugin_deps(rustc: &mut ProcessBuilder, build_state: &BuildMap, - build_scripts: &BuildScripts) + build_scripts: &BuildScripts, + root_output: &PathBuf) -> CargoResult<()> { let var = util::dylib_path_envvar(); let search_path = rustc.get_env(var).unwrap_or(OsString::new()); @@ -502,15 +506,47 @@ fn add_plugin_deps(rustc: &mut ProcessBuilder, let output = build_state.get(&key).chain_error(|| { internal(format!("couldn't find libs for plugin dep {}", id)) })?; - for path in output.library_paths.iter() { - search_path.push(path.clone()); - } + search_path.append(&mut filter_dynamic_search_path(output.library_paths.iter(), + root_output)); } let search_path = join_paths(&search_path, var)?; rustc.env(var, &search_path); Ok(()) } +// Determine paths to add to the dynamic search path from -L entries +// +// Strip off prefixes like "native=" or "framework=" and filter out directories +// *not* inside our output directory since they are likely spurious and can cause +// clashes with system shared libraries (issue #3366). +fn filter_dynamic_search_path<'a, I>(paths :I, root_output: &PathBuf) -> Vec + where I: Iterator { + let mut search_path = vec![]; + for dir in paths { + let dir = match dir.to_str() { + Some(s) => { + let mut parts = s.splitn(2, '='); + match (parts.next(), parts.next()) { + (Some("native"), Some(path)) | + (Some("crate"), Some(path)) | + (Some("dependency"), Some(path)) | + (Some("framework"), Some(path)) | + (Some("all"), Some(path)) => path.into(), + _ => dir.clone(), + } + } + None => dir.clone(), + }; + if dir.starts_with(&root_output) { + search_path.push(dir); + } else { + debug!("Not including path {} in runtime library search path because it is \ + outside target root {}", dir.display(), root_output.display()); + } + } + search_path +} + fn prepare_rustc(cx: &mut Context, crate_types: Vec<&str>, unit: &Unit) -> CargoResult { diff --git a/tests/build-script.rs b/tests/build-script.rs index 452358730..8d40474d2 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -1131,7 +1131,15 @@ fn test_dev_dep_build_script() { #[test] fn build_script_with_dynamic_native_dependency() { - let build = project("builder") + + let workspace = project("ws") + .file("Cargo.toml", r#" + [workspace] + members = ["builder", "foo"] + "#); + workspace.build(); + + let build = project("ws/builder") .file("Cargo.toml", r#" [package] name = "builder" @@ -1147,11 +1155,9 @@ fn build_script_with_dynamic_native_dependency() { #[no_mangle] pub extern fn foo() {} "#); - assert_that(build.cargo_process("build").arg("-v") - .env("RUST_LOG", "cargo::ops::cargo_rustc"), - execs().with_status(0)); + build.build(); - let foo = project("foo") + let foo = project("ws/foo") .file("Cargo.toml", r#" [package] name = "foo" @@ -1180,7 +1186,7 @@ fn build_script_with_dynamic_native_dependency() { fn main() { let src = PathBuf::from(env::var("SRC").unwrap()); - println!("cargo:rustc-link-search={}/target/debug/deps", + println!("cargo:rustc-link-search=native={}/target/debug/deps", src.display()); } "#) @@ -1192,8 +1198,13 @@ fn build_script_with_dynamic_native_dependency() { unsafe { foo() } } "#); + foo.build(); + + assert_that(build.cargo("build").arg("-v") + .env("RUST_LOG", "cargo::ops::cargo_rustc"), + execs().with_status(0)); - assert_that(foo.cargo_process("build").arg("-v").env("SRC", build.root()) + assert_that(foo.cargo("build").arg("-v").env("SRC", build.root()) .env("RUST_LOG", "cargo::ops::cargo_rustc"), execs().with_status(0)); } diff --git a/tests/plugins.rs b/tests/plugins.rs index 8ad26ba36..bcc2f7aff 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -90,7 +90,14 @@ fn plugin_to_the_max() { fn plugin_with_dynamic_native_dependency() { if !is_nightly() { return } - let build = project("builder") + let workspace = project("ws") + .file("Cargo.toml", r#" + [workspace] + members = ["builder", "foo"] + "#); + workspace.build(); + + let build = project("ws/builder") .file("Cargo.toml", r#" [package] name = "builder" @@ -105,16 +112,9 @@ fn plugin_with_dynamic_native_dependency() { #[no_mangle] pub extern fn foo() {} "#); - assert_that(build.cargo_process("build"), - execs().with_status(0)); - let src = build.root().join("target/debug"); - let lib = fs::read_dir(&src).unwrap().map(|s| s.unwrap().path()).find(|lib| { - let lib = lib.file_name().unwrap().to_str().unwrap(); - lib.starts_with(env::consts::DLL_PREFIX) && - lib.ends_with(env::consts::DLL_SUFFIX) - }).unwrap(); + build.build(); - let foo = project("foo") + let foo = project("ws/foo") .file("Cargo.toml", r#" [package] name = "foo" @@ -165,8 +165,19 @@ fn plugin_with_dynamic_native_dependency() { unsafe { foo() } } "#); + foo.build(); + + assert_that(build.cargo("build"), + execs().with_status(0)); + + let src = workspace.root().join("target/debug"); + let lib = fs::read_dir(&src).unwrap().map(|s| s.unwrap().path()).find(|lib| { + let lib = lib.file_name().unwrap().to_str().unwrap(); + lib.starts_with(env::consts::DLL_PREFIX) && + lib.ends_with(env::consts::DLL_SUFFIX) + }).unwrap(); - assert_that(foo.cargo_process("build").env("SRC", &lib).arg("-v"), + assert_that(foo.cargo("build").env("SRC", &lib).arg("-v"), execs().with_status(0)); } -- 2.30.2